Change tooltip window flags #7613

regulus79 · 2024-12-01T02:45:29Z

Partially addresses #7145.

This PR changes the window flags of TextFloats (the tooltip things which appear when loading samples/vst/etc) so that they do not stay on top of other windows.

Rossmaxx · 2024-12-12T11:02:56Z

obs-recording-2024-12-12-16-27.mp4

This is how the tooltips work now on my computer. Ignore the vst bug, it's been there for a while.
But at the start, the tooltip appears hidden. Is it intended?

Rossmaxx · 2024-12-12T11:06:06Z

I'll approve and merge once I get a clarification

regulus79 · 2024-12-12T22:57:41Z

the tooltip appears hidden. Is it intended?

This is not intended. But I don't know why it is happening. Given that I can see the faint outline of the tooltip window in your video before it appears probably means that the window has been created, but not yet filled? I don't know if that would be an issue with the windowing system or with Qt.

Rossmaxx · 2024-12-13T03:56:07Z

Thanks for the explanation. Your suspects of windowing system and qt seems valid. I believe such glitches are found here and there with qt applications on x11. Btw are you on x11 or Wayland?

Rossmaxx

Since it's improving the situation, LGTM.

regulus79 · 2024-12-13T04:00:42Z

Btw are you on x11 or Wayland?

I am on Wayland. I actually don't use/have any VSTs so I'm unsure if this particular issue occurs for me, but other times where TextFloats appear (like loading a sample) work fine.

Rossmaxx · 2024-12-13T04:03:01Z

On sample load, i don't even see a text float

regulus79 · 2024-12-13T04:05:02Z

When loading large samples, it appears for a split second.

michaelgregorius · 2024-12-16T18:05:46Z

To me it does not look like it is really improving the situation. It now just seems to show the message box that asks the user to wait until the action is completed much too late.

It would be better to find out what's causing the current behavior. To me it looks like there's some code that wants to show the message and which start doing so but then something long running is taking over the GUI main thread?

Rossmaxx · 2024-12-19T09:33:48Z

@regulus79 how much sense would it make to replace this TextFloat with a QToolTip directly?

michaelgregorius · 2024-12-19T14:09:19Z

@Rossmaxx, a QToolTip is shown interactively when a user hovers with the mouse over a widget whereas TextFloat is intended to be shown "programmatically". They are really different classes of widgets and should not be mixed IMO.

I have checked VestigeInstrument::loadFile and it is as expected. A call to gui::TextFloat::displayMessage is made and it is then immediately followed by potentially long running actions like loading and initializing the plugin on the GUI main thread. Hence it all happens during the processing of a single event and the text float cannot be shown before the long running actions have performed.

Can someone please check if it gets better with the following adjustment in gui::TextFloat::displayMessage (addition of QCoreApplication::processEvents()):

	if( gui::getGUI() != nullptr )
	{
		tf = gui::TextFloat::displayMessage(
				tr( "Loading plugin" ),
				tr( "Please wait while loading the VST plugin..." ),
				PLUGIN_NAME::getIconPixmap( "logo", 24, 24 ), 0 );
		
		QCoreApplication::processEvents();
	}

I don't any plugin that takes as much time as the one used by @Rossmaxx in the video.

Rossmaxx · 2024-12-19T14:14:13Z

Or nvm, that regression caught in my video might be the one from #7554, fixed by #7624. I do have to test now again to confirm. If it's so, there's nothing to do, I'll try @michaelgregorius' suggestion too, just to be sure.

The plug-ins i use are 4front e piano and dsk grand piano (IIRC, grabbed them from plugins4free)

michaelgregorius · 2024-12-19T15:23:40Z

I have just created #7626 which reports other problems with TextFloat under Wayland. Perhaps it is best to implement messages in a different way as proposed in the issue.

regulus79 · 2024-12-21T03:28:05Z

@michaelgregorius Although this PR does not fix the other issues with TextFloats, it does at least make it so that they do not stay on top of all other application windows, which I think is an improvement. I do agree that there may be better ways to implement this sort of message box than creating a new window, but would it be beneficial to merge this fix now and rework TextFloats later?

Rossmaxx · 2024-12-21T03:29:46Z

@michaelgregorius Although this PR does not fix the other issues with TextFloats, it does at least make it so that they do not stay on top of all other application windows, which I think is an improvement. I do agree that there may be better ways to implement this sort of message box than creating a new window, but would it be beneficial to merge this fix now and rework TextFloats later?

This is what I was telling. Though I wasn't descriptive with it.

…sage()

regulus79 · 2024-12-21T03:43:52Z

Can someone please check if it gets better with the following adjustment in gui::TextFloat::displayMessage (addition of QCoreApplication::processEvents()):

I tested this, and it does appear to fix the TextFloat being black at the start. I have now added QCoreApplication::processEvents() to the end of TextFloat::displayMessage. If other people could test this that would be great.

Edit: I'm not actually sure it fully fixed the issue; now it feels like the TextFloats appear later than they usually do. It's hard to tell.

michaelgregorius · 2024-12-21T09:54:01Z

src/gui/widgets/TextFloat.cpp

@@ -124,6 +125,8 @@ TextFloat * TextFloat::displayMessage(const QString & title,
 		QTimer::singleShot(timeout, tf, SLOT(close()));
 	}

+	QCoreApplication::processEvents();


IMO this is the wrong place to fix the problem. Instead the problem(s) should be fixed in the places where long running operations are performed in the GUI main thread, e.g. in the VST loading code, etc.

The TextFloat as is works perfectly okay and you would see similar problems with other widgets if they are about to be shown before something that blocks the GUI main thread.

The TextFloat as is works perfectly okay and you would see similar problems with other widgets if they are about to be shown before something that blocks the GUI main thread.

Then why can the problem be still observed when the TextFloat is shown without any blocking operations? If you switch workspaces while displaying a TextFloat, it will be drawn on top of the workspace.

Example: Holding down on a clip while switching workspaces.

Is the UI thread blocked/not running when not in view? Maybe that's why?

@sakertooth, I think we might be talking about different problems.

I am talking about the "garbage" that is sometimes shown in the TextFloat window, like for example shown in the first screenshot in this comment: #7145 (comment)

This seems to caused by the following chain of events:

Code instructs to show the TextFloat with a certain content/message. An event is put into the GUI event loop to show and paint the widget.

For some reason the TextFloat window is already shown but not painted yet, i.e. it shows garbage.

A long running operation runs in the main thread, e.g. the loading of a VST plugin, and it blocks the processing of further events. This results in the garbage being visible for a long time.

The long running operation completes and the paint event for the TextFloat is processed, i.e. it is now painted correctly. However, because the long running operation took so long it might even be scheduled for closing again which results in that the users have only seem garbage during that time.

By calling QCoreApplication::processEvents at the correct place before the long operation the chain changes as follows:

Code instructs to show the TextFloat with a certain content/message. An event is put into the GUI event loop to paint the widget.

The code calls QCoreApplication::processEvents before the long running operation. This results in the TextFloat being shown and painted correctly within milliseconds. For the users it is now shown correctly.

The long running operation runs in the main thread. It is still blocking the main thread from processing further events on time but the TextFloat is shown correctly during that time.

My argument is that calling QCoreApplication::processEvents in TextFloat is the wrong place because it must be called close to the code that "knows" that it is about to execute a long running operation on the GUI main thread, e.g. in the VST plugin code. The reason is that the problematic chain that's described above would also apply for any other widget to be shown before the long running operation. So if we wanted to show a push button is would likely also only contain garbage. However, we would never get the idea to extend the QPushButton with some obscure call to QCoreApplication::processEvents because it might be shown in such a long running context.

Rossmaxx · 2025-01-16T06:55:10Z

It's been some time since I've looked at this. IMO, it would be better to merge the tooltip fix and get done rn and do a follow up for the process events and threading fix. I'm telling this because even without the process event fix, the textfloat seems to "bleed out into the desktop" which does get fixed without the other stuff.

@michaelgregorius what do you think?

michaelgregorius · 2025-01-16T17:09:05Z

@Rossmaxx, do you mean to only leave the following line?

QWidget(getGUI()->mainWindow(), Qt::Tool | Qt::FramelessWindowHint)

And then to put the calls to QCoreApplication::processEvents at the right places in another PR?

That would be okay for me. But the call to QCoreApplication::processEvents in the TextFloat should definitively be removed as the text float itself is not creating the problems of the garbled display and therefore the wrong place to fix this.

…splayMessage()" This reverts commit b5f5379.

regulus79 · 2025-01-16T17:23:24Z

I have removed the call to QCoreApplication::processEvents() from TextFloat.

Change tooltip window flags

561cceb

regulus79 mentioned this pull request Dec 1, 2024

Disable this dialog when importing. #7145

Open

Rossmaxx linked an issue Dec 3, 2024 that may be closed by this pull request

Disable this dialog when importing. #7145

Open

Rossmaxx approved these changes Dec 13, 2024

View reviewed changes

Rossmaxx requested a review from michaelgregorius December 16, 2024 03:24

Add QCoreApplication::processEvents() to end of TextFloat::displayMes…

b5f5379

…sage()

michaelgregorius requested changes Dec 21, 2024

View reviewed changes

Revert "Add QCoreApplication::processEvents() to end of TextFloat::di…

a6f98e5

…splayMessage()" This reverts commit b5f5379.

michaelgregorius approved these changes Jan 16, 2025

View reviewed changes

Rossmaxx merged commit b21a269 into LMMS:master Jan 17, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change tooltip window flags #7613

Change tooltip window flags #7613

regulus79 commented Dec 1, 2024

Rossmaxx commented Dec 12, 2024

Rossmaxx commented Dec 12, 2024

regulus79 commented Dec 12, 2024

Rossmaxx commented Dec 13, 2024

Rossmaxx left a comment

regulus79 commented Dec 13, 2024

Rossmaxx commented Dec 13, 2024

regulus79 commented Dec 13, 2024

michaelgregorius commented Dec 16, 2024

Rossmaxx commented Dec 19, 2024 •

edited

Loading

michaelgregorius commented Dec 19, 2024

Rossmaxx commented Dec 19, 2024

michaelgregorius commented Dec 19, 2024

regulus79 commented Dec 21, 2024

Rossmaxx commented Dec 21, 2024 •

edited

Loading

regulus79 commented Dec 21, 2024 •

edited

Loading

michaelgregorius Dec 21, 2024

sakertooth Jan 4, 2025 •

edited

Loading

sakertooth Jan 4, 2025

michaelgregorius Jan 4, 2025

Rossmaxx commented Jan 16, 2025

michaelgregorius commented Jan 16, 2025

regulus79 commented Jan 16, 2025

Change tooltip window flags #7613

Change tooltip window flags #7613

Conversation

regulus79 commented Dec 1, 2024

Rossmaxx commented Dec 12, 2024

Rossmaxx commented Dec 12, 2024

regulus79 commented Dec 12, 2024

Rossmaxx commented Dec 13, 2024

Rossmaxx left a comment

Choose a reason for hiding this comment

regulus79 commented Dec 13, 2024

Rossmaxx commented Dec 13, 2024

regulus79 commented Dec 13, 2024

michaelgregorius commented Dec 16, 2024

Rossmaxx commented Dec 19, 2024 • edited Loading

michaelgregorius commented Dec 19, 2024

Rossmaxx commented Dec 19, 2024

michaelgregorius commented Dec 19, 2024

regulus79 commented Dec 21, 2024

Rossmaxx commented Dec 21, 2024 • edited Loading

regulus79 commented Dec 21, 2024 • edited Loading

michaelgregorius Dec 21, 2024

Choose a reason for hiding this comment

sakertooth Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

sakertooth Jan 4, 2025

Choose a reason for hiding this comment

michaelgregorius Jan 4, 2025

Choose a reason for hiding this comment

Rossmaxx commented Jan 16, 2025

michaelgregorius commented Jan 16, 2025

regulus79 commented Jan 16, 2025

Rossmaxx commented Dec 19, 2024 •

edited

Loading

Rossmaxx commented Dec 21, 2024 •

edited

Loading

regulus79 commented Dec 21, 2024 •

edited

Loading

sakertooth Jan 4, 2025 •

edited

Loading